Skip to content

Refactor script registry pattern - eliminate 27 sync.Once patterns#4768

Merged
pelikhan merged 6 commits intomainfrom
copilot/refactor-semantic-function-clustering
Nov 25, 2025
Merged

Refactor script registry pattern - eliminate 27 sync.Once patterns#4768
pelikhan merged 6 commits intomainfrom
copilot/refactor-semantic-function-clustering

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Nov 25, 2025

  • Analyze codebase structure and identify refactoring opportunities
  • Verify existing patterns are already consolidated:
    • ✅ PackageExtractor framework in package_extraction.go (used by npm.go, pip.go, dependabot.go)
    • BuildStandardNpmEngineInstallSteps for engine installation (used by Claude, Codex, Copilot)
    • buildStandardSafeOutputEnvVars for safe output jobs (used by 16+ files)
    • buildSafeOutputJob for safe output job scaffolding (used by 22+ files)
  • P4: Refactor script registry pattern - Implemented:
    • Created ScriptRegistry abstraction in script_registry.go (190 LOC)
    • Refactored scripts.go from 657 LOC to 270 LOC (-59% reduction)
    • Refactored js.go to use registry, removing ~40 LOC of boilerplate
    • Eliminated 27 repetitive sync.Once patterns
    • Net reduction: ~330 lines of duplicated code
    • Maintained full backward compatibility with existing getter functions
  • Run tests to verify no regressions
  • Address code review feedback
  • Run CodeQL security analysis - no issues found
  • Add test for overwrite behavior with Get() after bundling
Original prompt

This section details on the original issue you should resolve

<issue_title>[refactor] 🔧 Semantic Function Clustering Analysis - November 2025</issue_title>
<issue_description># 🔧 Semantic Function Clustering Analysis

Analysis Date: November 25, 2025
Repository: githubnext/gh-aw
Workflow Run: §19662572763

Executive Summary

Analyzed 238 non-test Go source files (65,911 lines of code) across the gh-aw repository using semantic analysis. The codebase demonstrates strong architectural patterns in validation files (14 dedicated validation files), configuration helpers, and engine organization. Key findings reveal high-value consolidation opportunities in safe output builders, package extraction patterns, and script loading mechanisms.

Key Findings:

  • Excellent organization: 14 validation files, clean engine structure, shared config helpers
  • 🎯 High-impact opportunities: 19 safe output files with 85-90% similar patterns
  • 💰 Quick wins: Package extraction already has framework, just needs adoption
  • 📊 Distribution: pkg/workflow (142 files, 60%), pkg/cli (75 files, 31%)
  • 🔄 Primary improvements: Safe output builders, script loading registry, engine installation
Full Refactoring Analysis Report

Repository Structure

Package Distribution

Package Files Percentage Primary Purpose
pkg/workflow 142 59.7% Workflow compilation, validation, execution
pkg/cli 75 31.5% CLI commands and user interactions
pkg/parser 12 5.0% Parsing utilities (YAML, frontmatter, GitHub)
pkg/console 4 1.7% Console output formatting
Others 5 2.1% Utilities (gitutil, logger, timeutil, constants)

Total: 238 non-test Go files, 65,911 lines of code


Well-Organized Patterns ✅

1. Validation Files (Exemplary Organization)

14 dedicated validation files demonstrate best-in-class separation of concerns:

pkg/workflow/agent_validation.go
pkg/workflow/bundler_validation.go
pkg/workflow/docker_validation.go
pkg/workflow/engine_validation.go
pkg/workflow/expression_validation.go
pkg/workflow/mcp_config_validation.go
pkg/workflow/npm_validation.go
pkg/workflow/pip_validation.go
pkg/workflow/repository_features_validation.go
pkg/workflow/runtime_validation.go
pkg/workflow/schema_validation.go
pkg/workflow/step_order_validation.go
pkg/workflow/strict_mode_validation.go
pkg/workflow/template_validation.go

✅ Each validation domain has its own file
✅ Clear, predictable naming convention
✅ Easy to locate and extend
✅ Follows "one file per feature" principle

2. Configuration Helpers (Model Pattern)

The file config_helpers.go is an exemplar of consolidation done right:

// pkg/workflow/config_helpers.go - 109 lines, widely reused:
parseLabelsFromConfig()           // Used across 19+ files
parseTitlePrefixFromConfig()      // Used extensively
parseTargetRepoWithValidation()   // Used 34+ times
parseParticipantsFromConfig()     // Used for assignees/reviewers
extractStringFromMap()            // Generic helper

Impact: These helpers eliminate ~500 lines of duplication across the codebase.

This is the model pattern that should be extended to other areas

3. Engine Implementation Files (Clean Architecture)

5 engine files with consistent structure:

pkg/workflow/agentic_engine.go (19KB - base engine and registry)
pkg/workflow/claude_engine.go (13KB)
pkg/workflow/codex_engine.go (24KB)
pkg/workflow/copilot_engine.go (35KB)
pkg/workflow/custom_engine.go (11KB)

Supporting files:

  • engine_helpers.go - Shared utilities
  • engine_validation.go - Engine validation
  • engine_output.go - Output handling
  • engine_network_hooks.go - Network hooks
  • engine_firewall_support.go - Firewall support

✅ One file per engine type
✅ Clear base interface pattern (BaseEngine)
✅ Appropriate helper file usage
✅ Good separation of concerns

4. Safe Output Files Pattern

19 safe output files follow consistent *_*.go pattern:

Create operations (6 files):

create_agent_task.go
create_code_scanning_alert.go
create_discussion.go
create_issue.go
create_pr_review_comment.go
create_pull_request.go

Close operations (3 files):

close_discussion.go
close_issue.go
close_pull_request.go

Other operations (10 files):

add_comment.go
add_labels.go
add_reviewer.go
assign_milestone.go
assign_to_agent.go
push_to_pull_request_branch.go
update_issue.go
update_project.go
update_release.go

✅ One file per safe output type
✅ Predictable structure within each file
✅ Easy to locate functionality

5. CLI Command Structure (Excellent)

CLI commands follow clear *_command.go pattern:

add_command.go
compile_command.go
init_command.go
list_command.go
pr_command.go
rem...

</details>

- Fixes githubnext/gh-aw#4732

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).

Copilot AI and others added 2 commits November 25, 2025 19:32
- Create ScriptRegistry abstraction in script_registry.go (190 LOC)
- Reduce scripts.go from 657 LOC to 270 LOC (-59%)
- Reduce js.go by ~40 LOC of boilerplate
- Net reduction: ~330 lines of duplicated code
- All 27 getter functions now use registry.Get()
- Lazy bundling with caching maintained
- Full backward compatibility with existing API

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
- Remove cross-file implementation detail comment
- Fix documentation accuracy (15 LOC per script, not 20)

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor semantic function clustering analysis results Refactor script registry pattern - eliminate 27 sync.Once patterns Nov 25, 2025
Copilot AI requested a review from mnkiefer November 25, 2025 19:44
@mnkiefer mnkiefer removed their assignment Nov 25, 2025
@mnkiefer mnkiefer requested review from pelikhan and removed request for mnkiefer November 25, 2025 20:08
@pelikhan pelikhan marked this pull request as ready for review November 25, 2025 20:09
Copilot AI review requested due to automatic review settings November 25, 2025 20:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR successfully eliminates 27 repetitive sync.Once patterns by introducing a centralized ScriptRegistry abstraction for lazy JavaScript script bundling. The refactoring reduces code by ~330 lines while maintaining full backward compatibility and improving maintainability.

Key Changes:

  • Introduces ScriptRegistry pattern to centralize lazy bundling logic with thread-safe caching
  • Refactors scripts.go from 657 to 270 lines (-59%) by replacing individual sync.Once patterns with registry registration
  • Refactors js.go by migrating 3 additional script patterns to the same registry system

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
pkg/workflow/script_registry.go New abstraction providing thread-safe lazy bundling with sync.Once semantics internally; includes Register() and Get() API with fallback error handling
pkg/workflow/script_registry_test.go Comprehensive test coverage including concurrent access, overwrite behavior, and edge cases
pkg/workflow/scripts.go Refactored to use ScriptRegistry; init() registers 26 scripts, getter functions reduced to one-line registry calls
pkg/workflow/js.go Refactored to use ScriptRegistry; init() registers 4 scripts (check_membership, safe_outputs_mcp_server, update_project, interpolate_prompt)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Comment thread pkg/workflow/script_registry_test.go
Adds TestScriptRegistry_Overwrite_AfterGet to verify that:
1. Get() triggers bundling on initial registration
2. Overwriting with new source replaces the entry
3. Get() returns bundled version of new source after overwrite

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 25, 2025

✅ Agentic Changeset Generator completed successfully.

@pelikhan pelikhan merged commit 8fa628b into main Nov 25, 2025
15 checks passed
@pelikhan pelikhan deleted the copilot/refactor-semantic-function-clustering branch November 25, 2025 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants